serve: name the HSA runtime HRX loads, path and sha256 (engine#713 criterion 4/5) - #888

#888 ready for review at f1605c2

PR Reviewer Guide 🔍

(Review updated until commit c507e90)

Here are some key observations to aid the review process:

if (!dev || !*dev || std::string(dev) == "hrx") {
    announce_hrx_runtime(o);
    announce_cwsr_env(o);
}

inline std::string sha256_hex(const uint8_t* data, size_t len) {
    static const uint32_t K[64] = {
        0x428a2f98, 0x71374491, 0xb5c0fbcf, 0xe9b5dba5, 0x3956c25b, 0x59f111f1, 0x923f82a4, 0xab1c5ed5,
        0xd807aa98, 0x12835b01, 0x243185be, 0x550c7dc3, 0x72be5d74, 0x80deb1fe, 0x9bdc06a7, 0xc19bf174,
        0xe49b69c1, 0xefbe4786, 0x0fc19dc6, 0x240ca1cc, 0x2de92c6f, 0x4a7484aa, 0x5cb0a9dc, 0x76f988da,
        0x983e5152, 0xa831c66d, 0xb00327c8, 0xbf597fc7, 0xc6e00bf3, 0xd5a79147, 0x06ca6351, 0x14292967,
        0x27b70a85, 0x2e1b2138, 0x4d2c6dfc, 0x53380d13, 0x650a7354, 0x766a0abb, 0x81c2c92e, 0x92722c85,
        0xa2bfe8a1, 0xa81a664b, 0xc24b8b70, 0xc76c51a3, 0xd192e819, 0xd6990624, 0xf40e3585, 0x106aa070,
        0x19a4c116, 0x1e376c08, 0x2748774c, 0x34b0bcb5, 0x391c0cb3, 0x4ed8aa4a, 0x5b9cca4f, 0x682e6ff3,
        0x748f82ee, 0x78a5636f, 0x84c87814, 0x8cc70208, 0x90befffa, 0xa4506ceb, 0xbef9a3f7, 0xc67178f2};
    uint32_t h[8] = {0x6a09e667, 0xbb67ae85, 0x3c6ef372, 0xa54ff53a, 0x510e527f, 0x9b05688c, 0x1f83d9ab, 0x5be0cd19};
    std::vector<uint8_t> m(data, data + len);
    m.push_back(0x80);
    while (m.size() % 64 != 56) m.push_back(0);
    for (int i = 7; i >= 0; i--) m.push_back(uint8_t((uint64_t(len) * 8) >> (8 * i)));
    auto rotr = [](uint32_t x, int n) { return (x >> n) | (x << (32 - n)); };
    for (size_t off = 0; off < m.size(); off += 64) {
        uint32_t w[64];
        for (int i = 0; i < 16; i++)
            w[i] = uint32_t(m[off + 4 * i]) << 24 | uint32_t(m[off + 4 * i + 1]) << 16 |
                   uint32_t(m[off + 4 * i + 2]) << 8 | m[off + 4 * i + 3];
        for (int i = 16; i < 64; i++) {
            const uint32_t s0 = rotr(w[i - 15], 7) ^ rotr(w[i - 15], 18) ^ (w[i - 15] >> 3);
            const uint32_t s1 = rotr(w[i - 2], 17) ^ rotr(w[i - 2], 19) ^ (w[i - 2] >> 10);
            w[i] = w[i - 16] + s0 + w[i - 7] + s1;
        }
        uint32_t a = h[0], b = h[1], c = h[2], d = h[3], e = h[4], f = h[5], g = h[6], hh = h[7];
        for (int i = 0; i < 64; i++) {
            const uint32_t t1 = hh + (rotr(e, 6) ^ rotr(e, 11) ^ rotr(e, 25)) + ((e & f) ^ (~e & g)) + K[i] + w[i];
            const uint32_t t2 = (rotr(a, 2) ^ rotr(a, 13) ^ rotr(a, 22)) + ((a & b) ^ (a & c) ^ (b & c));
            hh = g; g = f; f = e; e = d + t1; d = c; c = b; b = a; a = t1 + t2;
        }
        h[0] += a; h[1] += b; h[2] += c; h[3] += d; h[4] += e; h[5] += f; h[6] += g; h[7] += hh;
    }
    char hex[65];
    for (int i = 0; i < 8; i++) std::snprintf(hex + 8 * i, 9, "%08x", h[i]);
    return hex;
}

Thanks — two answers, and the first one is a real (if cosmetic) point.

static bool announced is not thread-safe. Correct in general, and it deliberately matches the neighbour it sits next to: announce_cwsr_env (app/serve.cpp:253-257) uses the identical pattern for the identical reason, and all three call sites are on serve's single-threaded launch/setup path — a lost or duplicated line here is a cosmetic outcome, not a wrong runtime (the path is hashed from the same hrx_libhsa() call the backend env gets). Converting both to a std::call_once/atomic would be the right cleanup; I did not do it in this PR so that it stays a two-line change and does not diverge from the CWSR line's shape. Happy to make it a follow-up if you prefer consistency the other way.

SHA-256 in a hot path: it runs once per serve process, at the first HRX backend, over a ~5 MB .so. Measured with this header, -O2: 24 ms for the pinned 5,141,585 B libhsa-runtime64.so.1 (digest 21a66824…, the same one the init line prints). No optimisation needed; the trade is ~1/40 of a second against being able to tell which of three runtimes a run used.

#888 ready for review at ebacdfa

(supersedes f1605c2: ci.yml now builds hrx_runtime_test — CI's build step lists every target a ctest test invokes, and mine was missing, so the first run reported "32 - hrx_runtime (Not Run)". Pushed over SSH because the gh OAuth token has no workflow scope.)

Persistent review updated to latest commit ebacdfa

#888 ready for review at e39b3fb

(ebacdfa6 was 9 commits behind main, so the branch now carries origin/main as a merge — no rebase, no
conflict, and the PR's own two commits are unchanged. Reason: this PR is on the c21 cut list, and a head that is
BEHIND would have needed the waiter or the owner to do the merge anyway.)

What I re-checked at the merged head:

• merge is textually clean (git merge-tree → 0 conflict hunks) and both submodule gitlinks are still the
  fleet's pins — third_party/hrx-system 82804197b, third_party/llama.cpp d8bd0ff6052a.
• the one real overlap is docs/hrx.md, and it does not collide: hrx: hrx-env names the HSA runtime, and a parity row compares CPU and device output (engine#674) #883 (now in main) added the
  trio/IREE_HAL_AMDGPU_LIBHSA_PATH paragraph in "The trio belongs to every HRX process"; build: the engine should fetch and pin its own HRX toolchain instead of requiring /opt/rocm-therock on the box #713's bullet is in the
  CMake "The HSA runtime" list. Same subject from the two ends (the recipe fills the path; serve names the file it
  actually opened and its sha256), no duplicated claim — I re-read both regions rather than trusting the clean merge.
• tools/copyright.py --check rc 0 (main also rewrote tools/copyright.py, so this was worth re-running).
• incremental build at the merged head: rc 0, and ctest -R 'hrx_runtime|hrx_cwsr|print_backend|mem_admit|npu_q4nx' → 8/8 pass (the two new tests plus the four
  regressions the extra log line could have broken). Under thermal-run, -j4; full CI is running on this head now.

Persistent review updated to latest commit e39b3fb

Persistent review updated to latest commit c507e90

#888 ready for review at c507e90

The waiter merged main (#885) into the branch itself, so this is a re-ping on the head it produced rather than a new
change from me. Verified at c507e90e:

• all checks green: build pass (5 m 11 s), pr_agent pass (59 s), Analyze ×4 pass, CodeQL pass, hw_gate
  skipping by design; mergeStateStatus: CLEAN, mergeable: MERGEABLE.
• the waiter's merge kept this PR's work intact: git diff e39b3fb..c507e90e is npu forward: read the batched prefill's GEMM outputs at the kernel's N stride (engine#855 part 2) #885's NPU files only (6 files,
  +377/−33), and app/hrx_runtime.h, tests/hrx_runtime_test.cpp, tests/hrx_runtime_route.sh are all present.
• my own local verification stands from the earlier comment (ctest -R 'hrx_runtime|hrx_cwsr|print_backend|mem_admit|npu_q4nx'
  → 8/8; tools/copyright.py --check rc 0; submodule gitlinks still hrx-system 82804197b, llama.cpp d8bd0ff6052a).

The earlier pr_agent red was its own concurrency cancel (conclusion: cancelled, empty step list), not a finding; it
passed on re-run and then passed again on this head.